Skip to content

Conversation

@s-barannikov
Copy link
Contributor

Same as SMUL, UMUL produces one result + flags, not two results + flags.

UMUL produces one result + flags, not two results + flags.
@llvmbot
Copy link
Member

llvmbot commented Nov 22, 2024

@llvm/pr-subscribers-backend-x86

Author: Sergei Barannikov (s-barannikov)

Changes

Same as SMUL, UMUL produces one result + flags, not two results + flags.


Full diff: https://github.com/llvm/llvm-project/pull/117377.diff

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrFragments.td (+2-7)
diff --git a/llvm/lib/Target/X86/X86InstrFragments.td b/llvm/lib/Target/X86/X86InstrFragments.td
index 038100b8264de7..ba9779cdc335d6 100644
--- a/llvm/lib/Target/X86/X86InstrFragments.td
+++ b/llvm/lib/Target/X86/X86InstrFragments.td
@@ -45,12 +45,7 @@ def SDTBinaryArithWithFlagsInOut : SDTypeProfile<2, 3,
                                              SDTCisInt<0>,
                                              SDTCisVT<1, i32>,
                                              SDTCisVT<4, i32>]>;
-// RES1, RES2, FLAGS = op LHS, RHS
-def SDT2ResultBinaryArithWithFlags : SDTypeProfile<3, 2,
-                                            [SDTCisSameAs<0, 1>,
-                                             SDTCisSameAs<0, 2>,
-                                             SDTCisSameAs<0, 3>,
-                                             SDTCisInt<0>, SDTCisVT<1, i32>]>;
+
 def SDTX86BrCond  : SDTypeProfile<0, 3,
                                   [SDTCisVT<0, OtherVT>,
                                    SDTCisVT<1, i8>, SDTCisVT<2, i32>]>;
@@ -266,7 +261,7 @@ def X86add_flag  : SDNode<"X86ISD::ADD",  SDTBinaryArithWithFlags,
 def X86sub_flag  : SDNode<"X86ISD::SUB",  SDTBinaryArithWithFlags>;
 def X86smul_flag : SDNode<"X86ISD::SMUL", SDTBinaryArithWithFlags,
                           [SDNPCommutative]>;
-def X86umul_flag : SDNode<"X86ISD::UMUL", SDT2ResultBinaryArithWithFlags,
+def X86umul_flag : SDNode<"X86ISD::UMUL", SDTBinaryArithWithFlags,
                           [SDNPCommutative]>;
 def X86adc_flag  : SDNode<"X86ISD::ADC",  SDTBinaryArithWithFlagsInOut>;
 def X86sbb_flag  : SDNode<"X86ISD::SBB",  SDTBinaryArithWithFlagsInOut>;

def X86smul_flag : SDNode<"X86ISD::SMUL", SDTBinaryArithWithFlags,
[SDNPCommutative]>;
def X86umul_flag : SDNode<"X86ISD::UMUL", SDT2ResultBinaryArithWithFlags,
def X86umul_flag : SDNode<"X86ISD::UMUL", SDTBinaryArithWithFlags,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just delete this line? It's not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather keep it. UMUL is created by X86ISelLowering and having a description in a td file would allow generating various info from it (a feature I'm currently working on but haven't created an RFC yet).

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@s-barannikov s-barannikov merged commit aa5dc53 into llvm:main Nov 23, 2024
8 of 10 checks passed
@s-barannikov s-barannikov deleted the sdag/x86-umul-type branch November 23, 2024 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants